Conversation
We already have this
|
|
||
| let crate_name = format_ident!("{}", crate_ident()?); | ||
| let class_name = get_class_name(&self_ty); | ||
| let module_name = format_ident!("__impl_exotic_{}__", class_name); |
There was a problem hiding this comment.
Should we consolidate this with the one in class to avoid duplication?
There was a problem hiding this comment.
I will rework this I am not satisfied. I think I can use the same trick we are using for the constructor with the double ref specialization.
| match crate::util::catch_unwind(f) { | ||
| Ok(x) => x, | ||
| Err(e) => unsafe { | ||
| self.get_opaque().set_panic(e); | ||
| qjs::JS_Throw(self.as_ptr(), qjs::JS_MKVAL(qjs::JS_TAG_EXCEPTION, 0)); | ||
| -1 | ||
| }, | ||
| } |
There was a problem hiding this comment.
This is exact duplicate of handle_panic. Can we use an inner method?
There was a problem hiding this comment.
I can check, dont remember why it was done like that.
There was a problem hiding this comment.
Kinda of in a crunch right now, I need to refactor the macro and the bool into an enum. If you want to help be my guess
| flags: qjs::c_int, | ||
| ) -> qjs::JSValue; | ||
|
|
||
| pub(crate) type GetPropertyFunc = for<'a> unsafe fn( |
There was a problem hiding this comment.
The for<'a> lifetime bound her seems unused and thus unnecessary.
| const CALLABLE: bool = false; | ||
|
|
||
| /// Is this class exotic (e.g. will exotic_* methods be called with it) | ||
| const EXOTIC: bool = false; | ||
|
|
There was a problem hiding this comment.
Since a class can't be both exotic and callable, it is probably better to encode that in the type system, i.e. change these bools to something like const KIND: ClassKind = ClassKind::Plain where ClassKind is something like:
enum ClassKind{
Plain,
Callable,
Exotic,
}There was a problem hiding this comment.
Right that is a good idea. Though I dont like the whole system of a single class ID for all rust classes.
4 participants
Finishes #497